Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove QUERY_AND_FETCH BWC for pre-5.3.0 nodes #25223

Merged
merged 3 commits into from
Jun 14, 2017

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Jun 14, 2017

This was a BWC layer where we expicitly set the search_type to
"query_and_fetch" when a single node is queried on pre-5.3 nodes. Since 6.0 no
longer needs to be compatible with 5.3 nodes, this can be removed.

This was a BWC layer where we expicitly set the `search_type` to
"query_and_fetch" when a single node is queried on pre-5.3 nodes. Since 6.0 no
longer needs to be compatible with 5.3 nodes, this can be removed.
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left two minor comments. Thanks for removing dead code!

request.searchType(SearchType.QUERY_AND_FETCH);
}
// TODO this BWC layer can be removed once this is back-ported to 5.3
transportService.sendChildRequest(connection, QUERY_FETCH_ACTION_NAME, request, task,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the QUERY_FETCH_ACTION_NAME constant still need to exist?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope! I removed those

// TODO this BWC layer can be removed once this is back-ported to 5.3
transportService.sendChildRequest(connection, QUERY_FETCH_ACTION_NAME, request, task,
new ActionListenerResponseHandler<>(listener, supplier));
} else {
transportService.sendChildRequest(connection, QUERY_ACTION_NAME, request, task,
new ActionListenerResponseHandler<>(listener, supplier));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's fix indentation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops yeah, I'll fix this.

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dakrone dakrone merged commit 4a30e23 into elastic:master Jun 14, 2017
@dakrone
Copy link
Member Author

dakrone commented Jun 14, 2017

Thanks @s1monw and @jpountz!

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 15, 2017
* master:
  Upgrade icu4j for the ICU analysis plugin to 59.1 (elastic#25243)
  move assertBusy to use CheckException (elastic#25246)
  Use SPI in High Level Rest Client to load XContent parsers (elastic#25098)
  [TEST] test that low level REST client leaves path untouched (elastic#25193)
  Speed up PK lookups at index time. (elastic#19856)
  [Docs] Fix documentation for percentiles bucket aggregation (elastic#25229)
  Upgrade to lucene-7.0.0-snapshot-92b1783. (elastic#25222)
  Build: Add master flag for disabling bwc tests (elastic#25230)
  Scripting: Rename SearchScript.needsScores to needs_score (elastic#25235)
  Support script context stateful factory in Painless. (elastic#25233)
  FastVectorHighlighter should not cache the field query globally (elastic#25197)
  Remove QUERY_AND_FETCH BWC for pre-5.3.0 nodes (elastic#25223)
  Add more missing AggregationBuilder getters (elastic#25198)
  Extract the snapshot/restore full cluster restart tests from the translog full cluster restart tests (elastic#25204)
jasontedor added a commit to glefloch/elasticsearch that referenced this pull request Jun 15, 2017
* master: (44 commits)
  Upgrade icu4j for the ICU analysis plugin to 59.1 (elastic#25243)
  move assertBusy to use CheckException (elastic#25246)
  Use SPI in High Level Rest Client to load XContent parsers (elastic#25098)
  [TEST] test that low level REST client leaves path untouched (elastic#25193)
  Speed up PK lookups at index time. (elastic#19856)
  [Docs] Fix documentation for percentiles bucket aggregation (elastic#25229)
  Upgrade to lucene-7.0.0-snapshot-92b1783. (elastic#25222)
  Build: Add master flag for disabling bwc tests (elastic#25230)
  Scripting: Rename SearchScript.needsScores to needs_score (elastic#25235)
  Support script context stateful factory in Painless. (elastic#25233)
  FastVectorHighlighter should not cache the field query globally (elastic#25197)
  Remove QUERY_AND_FETCH BWC for pre-5.3.0 nodes (elastic#25223)
  Add more missing AggregationBuilder getters (elastic#25198)
  Extract the snapshot/restore full cluster restart tests from the translog full cluster restart tests (elastic#25204)
  Refactor TransportShardBulkAction.executeUpdateRequest and add tests
  Make sure range queries are correctly profiled. (elastic#25108)
  Test: allow setting socket timeout for rest client (elastic#25221)
  Migration docs for elastic#25080 (elastic#25218)
  Remove `discovery.type` BWC layer from the EC2/Azure/GCE plugins elastic#25080
  When stopping via systemd only kill the JVM, not its control group (elastic#25195)
  ...
@clintongormley clintongormley added the :Search/Search Search-related issues that do not fall into other categories label Jun 19, 2017
@dakrone dakrone deleted the remove-bwc-query-compat branch December 13, 2017 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories v6.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants